ignore frames from frame collection logic #167
Conversation
beniwohli
force-pushed
the
ignore-frame-collection-frames
branch
2 times, most recently
from
3345df1
to
c0d7c11
Compare
elasticapm/utils/stacks.py
Outdated
| if not _getitem_from_frame(f_locals, '__traceback_hide__'): | ||
| f_globals = getattr(frame, 'f_globals', {}) | ||
| if not (f_globals.get('__name__', '').startswith(ignore_modules) | ||
| or _getitem_from_frame(f_locals, '__traceback_hide__')): |
elasticapm/utils/stacks.py
Outdated
| f_locals = getattr(frame, 'f_locals', {}) | ||
| f_globals = getattr(frame, 'f_globals', {}) | ||
| if not (f_globals.get('__name__', '').startswith(ignore_modules) | ||
| or _getitem_from_frame(f_locals, '__traceback_hide__')): |
elasticapm/utils/stacks.py
Outdated
| if not _getitem_from_frame(f_locals, '__traceback_hide__'): | ||
| f_globals = getattr(frame, 'f_globals', {}) | ||
| if not (f_globals.get('__name__', '').startswith(ignore_modules) | ||
| or _getitem_from_frame(f_locals, '__traceback_hide__')): |
elasticapm/utils/stacks.py
Outdated
| f_locals = getattr(frame, 'f_locals', {}) | ||
| f_globals = getattr(frame, 'f_globals', {}) | ||
| if not (f_globals.get('__name__', '').startswith(ignore_modules) | ||
| or _getitem_from_frame(f_locals, '__traceback_hide__')): |
beniwohli
force-pushed
the
ignore-frame-collection-frames
branch
2 times, most recently
from
98b7fdb
to
737c0c5
Compare
…ic#167) this should clean up the collected stack traces quite a bit Note, for Django this has been done a bit differently, this specialization has been removed. closes elastic#167
beniwohli
force-pushed
the
ignore-frame-collection-frames
branch
from
737c0c5
to
c353f9c
Compare
…ic#167) this should clean up the collected stack traces quite a bit Note, for Django this has been done a bit differently, this specialization has been removed. closes elastic#167
…ic#167) this should clean up the collected stack traces quite a bit Note, for Django this has been done a bit differently, this specialization has been removed. closes elastic#167
beniwohli
force-pushed
the
ignore-frame-collection-frames
branch
from
c353f9c
to
f857a0a
Compare
| while len(frames) > i: | ||
| if 'module' in frames[i] and not ( | ||
| frames[i]['module'].startswith('elasticapm.') or | ||
| frames[i]['module'] == 'contextlib' |
There was a problem hiding this comment.
what happens with contextlib, don't we want to filter it out anymore?
There was a problem hiding this comment.
I think that's a relic from the olden days. We don't use contextlib anywhere in the code base
| Frames can be skipped by either providing a number, or a tuple | ||
| of module names. If the module of a frame matches one of the names | ||
| (using `.startswith`, that frame will be skipped. This matching will only | ||
| be done until the first frame doesn't match. |
There was a problem hiding this comment.
i know you are doing this in a generic way, but for now you are only using it for the elasticapm.
in that case, don't you want to filter them out if even they are interleaved (like elasticapm.something -> database.something -> elasticapm.something_else)?
if that can not happen, why go through the troubles of implementing some logic you dont need?
There was a problem hiding this comment.
Glad you asked :D I actually intend to filter more than just elasticapm.. In #164, I introduce a functools.partial call. In CPython, this doesn't create a frame (I suppose because it's implemented in C? Not sure). But in PyPy, this creates a frame, with the module _functools. So my intention is to rebase that PR after merging this one, and then adding _functools to that list. But we only want to kill that one frame at the top, and not any functool.partial calls that come later on.
Another argument for not filtering out all frames that match is that this should only be used to cut away the top frames of the stack trace that are introduced by stack collection itself. If you call into any elasticapm code in other parts of the stack, that should IMO not be suppressed.
There was a problem hiding this comment.
that makes perfect sense. i am wiser now.
There was a problem hiding this comment.
there it is:
apm-agent-python/elasticapm/base.py
Lines 132 to 137 in 46e59e6
this should clean up the collected stack traces quite a bit Note, for Django this has been done a bit differently, this specialization has been removed. closes #167
this should clean up the collected stack traces quite a bit
Note, for Django this has been done a bit differently, this
specialization has been removed.
Before:
After: